Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Ensure hasComponentType checks fn value #4014

Merged
merged 1 commit into from
Nov 28, 2024
Merged

Conversation

DafyddLlyr
Copy link
Contributor

@DafyddLlyr DafyddLlyr commented Nov 27, 2024

What's the problem?

Invite to pay validation is only checking for the presence of a checklist, not a checklist with proposal.projectType set.

This means than an invalid flow can be configured, for which payment requests cannot be sent. I hit this when working on a follow up to #4010, see demo below.

main - incorrect behaviour

Screen.Recording.2024-11-27.at.18.22.50.mov

What's the cause?

The logic within hasComponentType() does not actually update the nodeIds array - it just maps and filters without reference to the source. As a result the fn check never actually happens, allowing any Checklist to pass the test.

Pizza - correct behaviour
Flow: https://4014.planx.pizza/testing/test-itp-validation

Screen.Recording.2024-11-27.at.19.18.27.mov

Comment on lines -422 to -430
const {
Checklist: _Checklist,
ChecklistOptionOne: _ChecklistOptionOne,
ChecklistOptionTwo: _ChecklistOptionTwo,
...invalidatedFlow
} = flowWithInviteToPay;
invalidatedFlow["_root"].edges?.splice(
invalidatedFlow["_root"].edges?.indexOf("Checklist"),
);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was passing as the checklist was entirely removed, it's now updated to have an incorrect data value.

Copy link

github-actions bot commented Nov 27, 2024

Removed vultr server and associated DNS entries

@DafyddLlyr DafyddLlyr requested a review from a team November 27, 2024 19:21
Copy link
Member

@jessicamcinchak jessicamcinchak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this fix !

@DafyddLlyr DafyddLlyr merged commit 7a523e9 into main Nov 28, 2024
12 checks passed
@DafyddLlyr DafyddLlyr deleted the dp/fix-fn-validation branch November 28, 2024 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants